Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with simd_bits +=. #633

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Conversation

fdmalone
Copy link
Contributor

@fdmalone fdmalone commented Sep 5, 2023

Caught this when trying to address #598.

@fdmalone
Copy link
Contributor Author

fdmalone commented Sep 5, 2023

Windows test is failing to build cirq?

@Strilanc
Copy link
Collaborator

Strilanc commented Sep 5, 2023

Looks like a hash failed to match. Probably transient, and if not then it's matplotlib's problem not yours.

@fdmalone
Copy link
Contributor Author

fdmalone commented Sep 8, 2023

For the fuzz tests I ran 10000 times initially. I also checked with the following python code:

from typing import List, Union
import numpy as np
from fixedint import UInt64

def join_words(words: List[int]) -> int:
    """Join a list of 64bit words into a single python integer"""
    out_str = ''
    for x in words[::-1]:
        out_str += f"{x:064b}"
    return int(out_str, 2)

def split_into_words(x: str) -> List[int]:
    """Split a binary representation of a python int into a list of 64 bit words"""
    strings = [x[i:i+64] for i in range(0, len(x), 64)]
    return [int(i, 2) for i in strings[::-1]]

def adder(lhs: List[UInt64], rhs: List[UInt64]) -> List[UInt64]:
    """Multi-word adder"""
    carry = 0
    for i, _ in enumerate(lhs):
        vbf = lhs[i]
        lhs[i] = lhs[i] + rhs[i] + carry
        carry = lhs[i] < vbf or (carry & (lhs[i] == vbf))
    return lhs


def to_fixed(x: int, width=192):
    """Fixed width string rep of python integer"""
    return f"{x:0{width}b}"[-width:]

def to_cpp_vec(x: List[Union[int, UInt64]], label="x"):
    """Write as a vector of integers"""
    out_str = f"std::vector<uint64_t> {label}{{"
    for i, w in enumerate(x):
        comma = "," if i < len(x) - 1 else ""
        out_str += f"{w}ULL{comma}"
    out_str += "};"
    print(out_str)

def gen_random_integer(num_bits):
    num_words = (num_bits - 1) // 64 + 1
    words = []
    for iw in range(num_words):
        r = np.random.random()
        b = 0
        if r < 1/3:
            b = 0
        elif r > 2/3:
            if iw == num_words - 1:
                for k in range(num_bits - 64 * iw):
                    b |= (1<< k)
            else:
                b = 0xFFFFFFFFFFFFFFFF
        else:
            for k in range(64):
                if np.random.random() > 0.5:
                    b |= (1<< k)
        words.append(b)
    return words

if __name__ == "__main__":
    np.random.seed(7)
    #for i in range(10_000):
    for i in range(4):
        #num_bits = np.random.randint(10, 1200) 
        num_bits = 512
        x = gen_random_integer(num_bits)
        y = gen_random_integer(num_bits)
        a = join_words(x)
        b = join_words(y)
        xpy = to_fixed(a+b, num_bits)
        xu = [UInt64(i) for i in x]
        yu = [UInt64(i) for i in y]
        z = adder(xu, yu)
        z_str = to_fixed(join_words(z), num_bits)
        xpy_str = xpy
        if i == 3:
            to_cpp_vec(x, "x")
            to_cpp_vec(y, "y")
            to_cpp_vec(split_into_words(xpy), "z")
        assert to_fixed(join_words(z), num_bits) == xpy
        assert xpy_str == z_str

@Strilanc
Copy link
Collaborator

Strilanc commented Sep 8, 2023

The tests appear to be crashing with buffer overflows.

@fdmalone
Copy link
Contributor Author

fdmalone commented Sep 8, 2023

The perils of developing on my mac

Copy link
Collaborator

@Strilanc Strilanc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the last round of review; feeling much more confident about it now.

src/stim/mem/simd_bits.test.cc Outdated Show resolved Hide resolved
src/stim/mem/simd_bits.test.cc Outdated Show resolved Hide resolved
src/stim/mem/simd_bits.test.cc Outdated Show resolved Hide resolved
@Strilanc Strilanc merged commit 0fdddef into quantumlib:main Sep 11, 2023
50 checks passed
@fdmalone fdmalone deleted the fix_bug_add_assignment branch September 11, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants